Repository navigation
Sweep core utils notice, npm_range, pep440, pipenv, purl - #1920
Mikola Lysenko (mikolalysenko) merged 3 commits into
Conversation
Assisted-by: Claude Code:claude-opus-5-5
Mikola Lysenko (mikolalysenko)
added
refactor
Inline notice's is_muted and pipenv's resolve_on_path, name npm's operator set once, and share pep440's trailing-zero trim. Drop the JsrPurlParts alias and the is_purl wrapper, and make parse_name_version private. Delete pipenv's three PATH-resolution tests (process.rs tests the shared resolver) and purl's builder, round-trip and is_purl tests, which restate a format! or repeat the parse tests. Fold purl's three test modules into one, and drop history and banner comments. Behavior is unchanged. Assisted-by: Claude Code:claude-opus-5-5
Each builder now round-trips through its parser with a literal + version, replacing the 14 deleted builder and round-trip tests with one that fails when a builder and its parser disagree. The pipenv probe's unused root parameter is named _root. Assisted-by: Claude Code:claude-opus-5-5
|
BugBot review Generated by Claude Code |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 9a0b930. Configure here.
Assisted-by: Claude Code:claude-opus-5-5
Mikola Lysenko (mikolalysenko)
added
the
Ready for review
|
[burn-down agent] Labeled Ready for review at
Generated by Claude Code |
Final review brief (
|
Wenxin Jiang (Wenxin-Jiang)
left a comment
Wenxin Jiang (Wenxin-Jiang)
left a comment
There was a problem hiding this comment.
[agent] Reviewed 9a0b930aaa3be4473d4ce5dca6ddbc71b1ea8d47. Checked notice suppression, npm range and PEP440 equivalence plus neutral pipenv probes. No major correctness issues found. Validation: source and regression-test review plus the available exact-head CI checks; no local test pass is claimed.
LLM Description written by Claude Code:claude-opus-5-5
Sweep of the audited core rows
utils/{notice,npm_range,package_manager,pep440,pipenv,purl}.rsincoverage/files.tsv, done by the architecture refactor routine (discussion #1830). See the living document's Part 4 (structure) and Part 5 (tests).Changes
notice.rs: inlinedis_mutedintonotice_once, its only caller.npm_range.rs: the operator character set was written out twice. It is now one privateis_operator.pep440.rs: the two copies of the trailing-zero trim are now onetrim_trailing_zeros. Dropped a bare#1589.pipenv.rs:resolve_on_path, a forwarder toprocess::resolve_tool_withover the real env. It now callsprocess::resolve_tool("pipenv").resolve_on_path_*) that repeatprocess.rs'sresolve_tool_*tests of the same resolver.rootparameter is now named_root, replacinglet _ = root. Removing it would touch callers inscan/hosted.rs(Sweep scan/hosted.rs (helpers and tests region) #1897) andvendor/pypi.rs(Fix pylock wiring ignoring an out-of-root -r pin (#1851) #1853, Fix templated pip pins ignored beside pylock (#1873) #1874, Fix Pipenv stale-install remedy built twice (#1878) #1883).purl.rs:JsrPurlPartsalias and theis_purlwrapper.target.rsnow callsstarts_with("pkg:")in both places.parse_name_versionis private.strip_purl_qualifiersessay, the JSR "as of writing" note, the builder banner, the REGRESSION/B73/removeandrollbackdon't PEP 503-normalise PyPI purl identifiers, soremove pkg:pypi/typing_extensions@4.7.1exits 1 "No patch found" whilegetaccepts the same identifier #1024 tags and 5 test section banners.test_is_purl, 7test_build_*_purltests (which restate aformat!), 7*_round_triptests andcargo_purl_round_trips_through_the_encoded_spelling(which repeat the parse tests). They are replaced by one test,builders_round_trip_through_their_parsers, which round-trips every builder with a literal+version. It went red whenbuild_jsr_purlwas mutated to drop the/.test_parse_golang_purl_percent_decodesandtest_parse_jsr_purl_percent_encoded_scopewere duplicates; their one unique assert each (Azure case is kept; an encoded bare@is rejected) moved into the parse tests.package_manager.rs: nothing local. Theyarn_majornote is A13, and its callers' files (vendor/mod.rs) are held by Fix Go patch delivery, audits, and reinstalls #1698.Not done because the files are held by open PRs: inlining the 7
build_*_purlwrappers (callers in crawlers and vendor files held by #1882, #1886, #1891 and #1698), inliningparse_jsr_purlintodeno_crawler(#1891), and moving the PurlKey asserts topurl_key.rs(#1907).Size
+135 / −432 in 6 files: about −55 production lines and −242 test lines.
Behavior change
None.
/code-review high
7 findings. I fixed 2: the builder/literal-
+round-trip coverage, and_root. I left 5 unchanged:npm_range_admits(">=", v)reads as*: pre-existing, and fixing it changes behavior. It is noted in the register for an issue..batspawn test went withpipenv.rs:process.rstests PATHEXT resolution and the batch-shim spawn with.cmd. A.batcase belongs there, butprocess.rsis held by Fix env-keyed probe memo written three ways (#1888) #1895.pub fn is_purlis a breaking change: core is consumed only by this workspace's CLI, and earlier sweeps narrowedpubitems the same way.starts_with("pkg:")is case-sensitive: identical to the oldis_purl.starts_with("pkg:"): a one-token literal; a wrapper for it is what the audit asked to remove.Verification
cargo clippy --workspace --all-features -- -D warningsis clean.cargo test -p socket-patch-core --lib: 6975 passed. The 4 failures are permission-dependent tests that fail under the root sandbox and are untouched here:copy_tree::relax_loop_must_not_traverse_symlinked_root,vlt_heal::an_unremovable_hidden_lock_keeps_every_store_entry,pypi_poetry::wire_write_failure_maps_error_and_leaves_lock_untouchedandpypi_requirements::wire_failure_rolls_back_already_written_files.9a0b930.Risk
Low. The changes are local inlines and test edits. No platform-specific production code changed; only a
cfg(windows)test was deleted.https://claude.ai/code/session_011f9aeQHPtUvnVjDnuWKCpj
Generated by Claude Code